Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add protocol option to ssl-context #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add protocol option to ssl-context #85

wants to merge 1 commit into from

Conversation

leek5pin
Copy link

When setting up the ssl-context I need to use TLSv1.2 or my request will fail. That is an optional parameter in the setup through the java api, but is not in your http async client. As result, on creation of the ssl-context it defaults to TLS. The proposed change would add an optional :protocol key to the ssl-context function which would default to TLS when not present, but could be specified.

@leek5pin leek5pin requested a review from cch1 February 14, 2020 14:47
Copy link
Owner

@cch1 cch1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test would be the icing on the cake. The implementation looks pretty simple and I'll pull as soon as you can demonstrate in a test that the protocol can be configured. There are already tons of tests so you should be able to find a good starting point.

@cch1
Copy link
Owner

cch1 commented Feb 19, 2020

@sean-stockholm Any chance you can add a test?

@leek5pin
Copy link
Author

I've been swamped with moving. I'll find some time to make a test before EOD Monday.

@cch1
Copy link
Owner

cch1 commented Oct 31, 2022

I've been swamped with moving. I'll find some time to make a test before EOD Monday.

Which Monday? :)

I'm in a good place to merge this PR if you can get a test in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants